Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Flatten project structure created by crystal init #6317

Merged
merged 1 commit into from Jul 3, 2018
Merged

Flatten project structure created by crystal init #6317

merged 1 commit into from Jul 3, 2018

Conversation

straight-shoota
Copy link
Member

@straight-shoota straight-shoota commented Jul 2, 2018 •
edited

crystal init previously created src/example/version.cr file which only contained a VERSION constant. This is moved into the main library file. It also removes require "./example/*" from main file.

The old version.cr convention persisted from Ruby due to the need for gemspec files to require only the version constant from the library.

Extracted from #5428 to make this easier to merge with (hopfully) less discussion and we can focus there (or in a new issue) on the value of VERSION. @RX14 I hope you don't mind 馃槈

#5428 (comment)

In my view version.cr is much more cluttered than just adding a line in your main file. Ruby convention has it separate only because the gemspec depends on the version.cr - in Crystal we have it the other way around so we can merge it back in.

@RX14 @straight-shoota
Flatten project structure created by crystal init
b8ca749 
`crystal init` previously created `src/example/version.cr` file which
only contained a `VERSION` constant. This is moved into the main library file,
The old `version.cr` convention persisted from ruby due to the need for
gemspec files to require only the version constant from the library.
ysbaddaden
ysbaddaden approved these changes Jul 2, 2018
View reviewed changes
Copy link
Contributor

@ysbaddaden ysbaddaden left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to put everything under src, not the artificial src/foo folder inherited from rubygem. I'm all in for this evolution.

@jkthorne
Copy link
Contributor

jkthorne commented Jul 2, 2018

I cant 馃憤 @ysbaddaden comment but I agree.

@kostya
Copy link
Contributor

kostya commented Jul 2, 2018

put everything in src produce much more require conflicts, as src is a search path in crystal.

@straight-shoota
Copy link
Member Author

straight-shoota commented Jul 2, 2018 •
edited

@kostya What? require "./shardname/foo" results in exactly the same naming issues (if you can call it such) as require "./foo" if everything is just put into src/shardname/ instead of src/

@ysbaddaden
Copy link
Contributor

@kostya no, this conflict only happens in Rubygem, which adds each gem lib folder to the search path, which forces to use a lib/foo folder. This doesn't happen in Crystal, where only the project's lib folder is added, not each src folder of every shard. Having a src/foo folder or not is exactly the same.

@icyleaf
Copy link
Contributor

icyleaf commented Jul 3, 2018

In my view version.cr is much more cluttered than just adding a line in your main file. Ruby convention has it separate only because the gemspec depends on the version.cr - in Crystal we have it the other way around so we can merge it back in.

I agree with @RX14 opinion, Is there have a better solve to combine VERSION in src and version in shard.yml, like keep last one to call Shards.version("name") .

@RX14 RX14 merged commit ae32184 into crystal-lang:master Jul 3, 2018
@RX14 RX14 added this to the Next milestone Jul 3, 2018
@straight-shoota straight-shoota deleted the jm/feature/crystal-init-remove-version.cr branch July 3, 2018 12:00
@jkthorne jkthorne mentioned this pull request Jul 20, 2018
Open
@RX14 RX14 modified the milestones: Next, 0.26.0 Jul 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ysbaddaden ysbaddaden ysbaddaden approved these changes

@jhass jhass jhass approved these changes

@RX14 RX14 RX14 approved these changes

Assignees
No one assigned
Labels
kind:refactor topic:compiler topic:tools
Projects
None yet
Milestone
0.26.0
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@straight-shoota @jkthorne @kostya @ysbaddaden @icyleaf @jhass @RX14