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.

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

Already on GitHub? Sign in to your account

Copilot's timestamp causes unnecessary merge conflicts in git. #3485

Closed
6TELOIV opened this issue Oct 17, 2024 · 5 comments
Closed

Copilot's timestamp causes unnecessary merge conflicts in git. #3485

6TELOIV opened this issue Oct 17, 2024 · 5 comments
Assignees

Comments

@6TELOIV
Copy link
Contributor

6TELOIV commented Oct 17, 2024

I'm submitting a

[x] feature request

...about

[x] Content Types or data management
[x] other / unknown

Current Behavior / Expected Behavior

When generating types with Copilot, the file contains a timestamp on line 17 and 27, even if nothing has changed related to the type.

This causes unnecessary conflicts when managing the source code for a 2sxc app on git. If 2 people are working on different features at the same time, both requiring different changes to different content types on the app, when trying to merge their changes, you will run into merge conflicts and manually need to assess resolve the issue (usually straightforward, but everyone hates an unnecessary conflict 😅)

I think the desired behavior is either:

  • Remove the timestamps entirely. Just specifying Generated. Re-generate whenever you change the ContentType. might be enough info for developers to not be confused. The timestamp is also kinda redundant, as git history should be a better source of history of work when trying to investigate issues.
  • Don't modify files if their generated code is unchanged. A naive way to do so would be generate the file content into a string, read the text of the existing file (if any exists), compare the contents, and if they are unchanged (sans the timestamps on 17 and 27), then don't write it to file. There could definitely be a better way to do this, maybe by storing the codegen metadata (like version generated with and when it was generated) on the content type itself as metadata, and then when running copilot generate, check when the content type and it's fields were modified, and if they are all before the codegen metadata time, skip it.

Instructions to Reproduce the Problem

  1. Initialize a Git repo in a 2sxc app folder
  2. Create 2 content types and generate code with copilot
  3. Create 2 branches
  4. Modify a different content type on each branch and generate code with copilot
  5. Merge both branches, and observe that even though each branch only modified one content type, both content types are touched and a merge conflict occurs.

Your environment

  • 2sxc version(s): 2sxc v.17.09.00
@iJungleboy
Copy link
Contributor

I'll need to think about this.

I understand the challenge and have had it myself, but the timestamp does help to see how up-to-date the generate file is.

@6TELOIV
Copy link
Contributor Author

6TELOIV commented Oct 18, 2024

I think the timestamp is OK as long as the file is only generated/modified when the contents would change. Either through a 2sxc update, or change to the content type.

Because if I had 2 branches that did modify the same content type, I would expect a merge conflict. And the desired solution would probably be pull one branch into the other, merging the app.xml state (at least with regards to content type changes) but keeping either of the co-pilot files. Then import the new app state on a local 2sxc instance, regen the code with copilot, and commit.

In the other case, where 2 branches did not modify the same content type, the app.xml should merge much easier, and there would be no conflicts on the timestamps (as both files would have different timestamps)

@6TELOIV
Copy link
Contributor Author

6TELOIV commented Oct 18, 2024

I think an alternative is to default to .gitignore the AppCode/Data/*.Generated.cs files, and then only run copilot as part of the installation process.

I think at some point if these copilot data files were auto generated by 2sxc whenever content types changed, instead of by developer action, then this would 100% be the correct solution, as they are no longer part of the "definition" of the app, and don't need to be stored in the repository.

@iJungleboy
Copy link
Contributor

improved in v18.03

will now just create a log file, and not add timestamps any more.

Of course if you had generated code, the next generate will be different, because the timestamp will be removed...

@6TELOIV
Copy link
Contributor Author

6TELOIV commented Nov 6, 2024

That sounds like an excellent solution to the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: v18.03
Development

No branches or pull requests

3 participants